-
-
Notifications
You must be signed in to change notification settings - Fork 34k
gh-140550: allow slots that repeat information from PyModuleDef #144340
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
When integrating slots-based module creation is with the inittab, which currently requires PyModuleDef, it would be convenient to reuse the the same slots array for the MethodDef. Allow slots that match what's already present in the PyModuleDef.
|
I can confirm that with this PR the PyO3 tests all pass on my |
| members, the values must match. | ||
| For example, if you use :c:macro:`Py_mod_name` in :c:member:`!m_slots`, | ||
| :c:member:`PyModuleDef.m_name` must be set to the same pointer | ||
| (not just an equal string). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"set to the same pointer" is a strong constraint which is difficult to reach in C. For example, your test copies the string "test_capi/parrot", and rely on the compiler to merge equal strings to use the same pointer. Would it be possible to use strcmp() for the name and the docstring?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, thanks for the catch; I meant to fix that.
The issue is, where do you draw the line? Do m_methods & Py_mod_methods also need to be only “equal”? And the names of the methods contained there?
I'd rather keep this simple. After all, it's only a workaround.
From feedback on the discussion thread:
When integrating slots-based module creation is with the inittab, which currently requires PyModuleDef, it would be convenient to reuse the the same slots array for the MethodDef.
This PR allows slots that match what's already present in the PyModuleDef.
cc @ngoldbaum
📚 Documentation preview 📚: https://cpython-previews--144340.org.readthedocs.build/